Misc Fixes For Help Forum#2337
Merged
Merged
Conversation
The current warning log includes the thread name, which means the log message varies wildly between threads. This causes issues with sentry since the actual error message gets trimmed, and sentry fails to group issues from this log as they appear as different messages. Signed-off-by: Hassan Abouelela <hassan@hassanamr.com>
When discord sends us the thread create event in help channels, it is not ready to perform other operations on the thread such as getting or pinning messages. This causes it to error out when we try to do these actions and claim that those channels don't exist. Instead, we sleep for a short time to try and wait for it to be ready. Signed-off-by: Hassan Abouelela <hassan@hassanamr.com>
We try to remove the cooldown role from users before checking if the user is still in the server, which can cause an error since the thread object will just contain `None` as the user. Signed-off-by: Hassan Abouelela <hassan@hassanamr.com>
The old method for detecting deleted opener messages does not seem to work, probably because the message is fetched from a cache or similar. Instead we simply try/except pinning the message and pass if the pinning failed. Signed-off-by: Hassan Abouelela <hassan@hassanamr.com>
The wait_for_deletion utility would try to remove reactions from a message after the timeout expires, which would normally be fine. In threads however, they can be closed while waiting for the timeout to expire. In such a case, the bot will try to remove the reactions after the channel has been closed and fail. A special exception was added for this case to do nothing, since this is only a QoL feature. Signed-off-by: Hassan Abouelela <hassan@hassanamr.com>
HassanAbouelela
added
t: bug
Similar to 555ed4e, the pagination utility needs to catch when it's trying to act on an archived thread. Signed-off-by: Hassan Abouelela <hassan@hassanamr.com>
In the case of an image or other media as the starter message, the formatted message in the help DM will be empty, which is invalid for the embed. We populate the field with some more useful text in this case. Signed-off-by: Hassan Abouelela <hassan@hassanamr.com>
HassanAbouelela
force-pushed
the
help-forum-fixes
branch
from
bf7d452 to
4ca04f6
Compare
ChrisLovering
approved these changes
Nov 26, 2022
shtlrs
reviewed
Nov 26, 2022
Contributor
shtlrs
left a comment
shtlrs
left a comment
There was a problem hiding this comment.
Looks good, just a couple of comments
| # Discord sends the open event long before the thread is ready for actions in the API. | ||
| # This causes actions such as fetching the message, pinning message, etc to fail. | ||
| # We sleep here to try and delay our code enough so the thread is ready in the API. | ||
| await asyncio.sleep(2) |
Contributor
There was a problem hiding this comment.
Are we sure that 2 seconds will always be enough ? Isn't there another event we can subscribe to, like the starter message for example ?
Member
There was a problem hiding this comment.
Yea, theoretically we could wait_for an on_message event which has the same message_id as the just created thread. Since this would then mean the message was sent to us via the gateway, and is also cached in d.py
mbaruh
reviewed
Nov 26, 2022
HassanAbouelela
force-pushed
the
help-forum-fixes
branch
2 times, most recently
from
79eb97d to
2857424
Compare
mbaruh
reviewed
Nov 26, 2022
| raise | ||
|
|
||
| try: | ||
| await message.delete() |
Member
There was a problem hiding this comment.
Theoretically the thread could get locked between the reaction and this line
Contributor
Author
There was a problem hiding this comment.
Refactored it again
Co-authored-by: Boris Muratov <8bee278@gmail.com> Co-authored-by: Amrou Bellalouna <48383734+shtlrs@users.noreply.github.com> Signed-off-by: Hassan Abouelela <hassan@hassanamr.com>
HassanAbouelela
force-pushed
the
help-forum-fixes
branch
from
2857424 to
8cfa7ed
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR includes a few small fixes for the help forum. Each commit includes descriptions of the issue, and the fix.